Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use as rather than map and discard #3708

Merged
merged 1 commit into from
Dec 8, 2020
Merged

Use as rather than map and discard #3708

merged 1 commit into from
Dec 8, 2020

Conversation

johnynek
Copy link
Contributor

@johnynek johnynek commented Dec 8, 2020

I noticed a few places where we do map(f)(_ => a). This is the same as Functor.as, but it hides it from the typeclass implementation. Some types can optimize as by discarding any trailing map operators from their internal AST (such as parsers or other compute types that are not evaluated until later). By not calling typeclass methods we potentially slow those implementations down.

@codecov-io
Copy link

codecov-io commented Dec 8, 2020

Codecov Report

Merging #3708 (26e16e5) into master (8e8be51) will not change coverage.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master    #3708   +/-   ##
=======================================
  Coverage   90.24%   90.24%           
=======================================
  Files         391      391           
  Lines        8863     8863           
  Branches      270      227   -43     
=======================================
  Hits         7998     7998           
  Misses        865      865           

Copy link
Member

@rossabaker rossabaker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A scalafix linter for patterns like this, or .pure(()), would be neat.

@johnynek johnynek merged commit 7fd119d into master Dec 8, 2020
@larsrh larsrh deleted the oscar/use_as_more branch January 30, 2021 12:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants